ARTEMIS-2189 allow deleting temp dest when session is closed #2448
Conversation
| @@ -265,6 +266,126 @@ public void testTemporaryQueueDeleted() throws Exception { | |||
| } | |||
| } | |||
|
|
|||
| @Test | |||
| public void testTemporaryQueueDeletedAfterSessionClosed() throws Exception { | |||
There was a problem hiding this comment.
Can the behaviour be checked with amqp and openwire to check impl behaviour is the same
There was a problem hiding this comment.
For AMQP the handling of dynamic nodes is tested here:
https://github.com/apache/activemq-artemis/blob/master/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpTempDestinationTest.java
And here:
https://github.com/apache/activemq-artemis/blob/master/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSTemporaryDestinationTest.java
For OpenWire you'd want to look for a test of the RemoveInfo command that carries a DestintionInfo object.
Other tests that are specifically targeted the JMS spec (or other) compliant behaviour of a client really belong to the project that maintains the client in question.
There was a problem hiding this comment.
The issue here is not with the broker but with the core client. Therefore, I'm not sure it makes sense to test other clients on this particular use-case.
| if (session.getCoreSession().isClosed()) { | ||
| // Temporary queues will be deleted when the connection is closed.. nothing to be done then! | ||
| return; | ||
| /** |
There was a problem hiding this comment.
@jbertram This code is still a bit racey in that the session could become closed right after checking that it isn't and thereby still allow a violation in what the spec says should be possible. It might make more sense to just use the approach of creating a session for this operation each time given this isn't something people are going to be doing a great deal and so doesn't need to be highly performant.
There was a problem hiding this comment.
Fair enough. I pushed an update.
There was a problem hiding this comment.
@tabish121 The updated version is fine mate? If is fine I will merge it 👍
There was a problem hiding this comment.
Looks ok to me now, shouldn't be racing with session close now.
jbertram
force-pushed
the
ARTEMIS-2189
branch
from
ff41765
to
39bf16a
Compare
No description provided.